fix(cli): error messages in utils/dlx/#1256
Merged
John-David Dalton (jdalton) merged 4 commits intomainfrom Apr 24, 2026
Merged
fix(cli): error messages in utils/dlx/#1256John-David Dalton (jdalton) merged 4 commits intomainfrom
John-David Dalton (jdalton) merged 4 commits intomainfrom
Conversation
Rewrites error messages across packages/cli/src/utils/dlx/ to follow
the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md.
Sources:
- spawn.mts: 27 messages
- 6x 'Unexpected resolution type for <tool>' — now name the resolver
function and the actual resolution.type seen
- Archive/platform errors name the supported formats/platforms
- Python DLX errors surface the lock file path and cache dir
- PyPI fetch errors include the URL that failed
- Security errors (zip-slip, symlink escape) tell user to delete
the cached asset and report upstream
- resolve-binary.mts: 4 messages (socket-patch, trivy, trufflehog,
opengrep platform support) — each now lists supported platforms
and suggests how to install the tool manually
- vfs-extract.mts: 5 messages (SEA VFS extraction failures) — each
names what went wrong with the bundle and how to recover
(usually: rebuild SEA)
Internal invariant errors stay as plain Error (not InputError) but
are informative enough that if they ever fire, the user can open
a useful bug report.
Tests updated: test/unit/utils/dlx/resolve-binary.test.mts (1
substring match switched to regex).
Follows strategy from #1254. Part of the multi-PR series started
by #1255 (commands/).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Error references variables before their declaration
- Moved the declarations of pythonDir, pythonBin, and lockFile above the retryCount >= MAX_RETRIES guard so they are initialized before the error message interpolates them, avoiding the temporal dead zone ReferenceError.
Or push these changes by commenting:
@cursor push 2a82947a44
Preview (2a82947a44)
diff --git a/packages/cli/src/utils/dlx/spawn.mts b/packages/cli/src/utils/dlx/spawn.mts
--- a/packages/cli/src/utils/dlx/spawn.mts
+++ b/packages/cli/src/utils/dlx/spawn.mts
@@ -1054,6 +1054,9 @@
*/
export async function ensurePythonDlx(retryCount = 0): Promise<string> {
const MAX_RETRIES = 3
+ const pythonDir = getPythonCachePath()
+ const pythonBin = getPythonBinPath(pythonDir)
+ const lockFile = path.join(pythonDir, '.downloading')
if (retryCount >= MAX_RETRIES) {
throw new InputError(
@@ -1061,10 +1064,6 @@
)
}
- const pythonDir = getPythonCachePath()
- const pythonBin = getPythonBinPath(pythonDir)
- const lockFile = path.join(pythonDir, '.downloading')
-
if (!existsSync(pythonBin)) {
await safeMkdir(pythonDir, { recursive: true })You can send follow-ups to the cloud agent here.
The previous commit referenced `${lockFile}` and `${pythonDir}` in
the MAX_RETRIES error message, but those consts were declared AFTER
the retry check. Hitting the retry path threw ReferenceError from
the temporal dead zone instead of the intended InputError.
Fix: move the three const declarations (pythonDir, pythonBin,
lockFile) above the MAX_RETRIES guard. Caught by Cursor bugbot
(#1256 (comment))
and confirmed by the type-check job.
1 task
Switch `(e as Error).message` to `e instanceof Error ? e.message : String(e)` so that when a non-Error value is thrown (strings, objects, null) the error message stays informative instead of becoming 'undefined'. Same fix as applied to #1260 (iocraft.mts) after Cursor bugbot flagged the pattern on that PR.
Switch to shared fleet helpers so error lists render as human prose
('a, b, and c') and error-cause stringification works safely for
non-Error throws (falls back to 'Unknown error' instead of crashing
or producing 'undefined').
- resolve-binary.mts: SOCKET_PATCH_ASSETS + OPENGREP_ASSETS
platform lists now use `joinAnd(Object.keys(...))`.
- vfs-extract.mts: missingTools list uses joinAnd; extract-failure
error now uses getErrorCause(e) — equivalent to the inline
'e instanceof Error ? e.message : String(e)' with the standard
UNKNOWN_ERROR fallback.
- spawn.mts: output-map listing uses joinAnd.
No behavior change for Error throws; non-Error throws now produce
'Unknown error' instead of '[object Object]' or similar.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7c72686. Configure here.
James Tu (jmsjtu)
approved these changes
Apr 24, 2026
This was referenced Apr 24, 2026
2 tasks
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 24, 2026
…rary migration (#1257) * fix(cli): align utils/update/ + utils/command/ error messages with 4-ingredient strategy Rewrites error messages across packages/cli/src/utils/update/ and packages/cli/src/utils/command/ to follow the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md. Sources: - utils/update/checker.mts: 8 messages (URL validation, package name / registry URL validation, version-response validation). Each now names the function, the received type, and what a valid value looks like. - utils/update/manager.mts: 3 messages (mirror guards for name / version / ttl). Still warn-and-return-false, but the text now tells the caller exactly which option was wrong. - utils/command/registry-core.mts: 6 messages (command / alias registration conflicts, middleware next() misuse, flag parsing failures). Each now names the offending command, flag name, or index so debuggers don't need to read source. Tests updated: - test/unit/utils/update/checker.test.mts: 6 assertions (switched to regex) - test/unit/utils/update/manager.test.mts: 3 assertions (switched to expect.stringContaining) - test/unit/utils/command/registry-core.test.mts: 5 assertions All 153 tests in the affected suites pass. Follows strategy from #1254. Part of the multi-PR series started by #1255 (commands/) and continued in #1256 (utils/dlx/). * fix(cli): address Cursor bugbot findings on error messages Two issues flagged by Cursor bugbot on #1257: 1. (Medium) registry-core.mts middleware error reported the wrong offending middleware index. `index` tracks the highest dispatched position, not the caller; when dispatch(i) is re-entered after a double-next(), the offender held middleware[i - 1]'s next closure. Fixed to use `i - 1` with a comment explaining why. 2. (Low) checker.mts error referenced a non-existent `UpdateChecker.fetch()` — the object is actually exported as `NetworkUtils`. Renamed in both the error and its test regex. Caught by #1257 bugbot review. * docs(claude): align Error Messages with fleet doctrine, add references doc Restructure the CLI-specific Error Messages section to match the updated fleet doctrine from socket-repo-template: - Keep the four ingredients (What / Where / Saw vs. wanted / Fix). - Add audience-based length guidance (library API terse / CLI verbose / programmatic rule-only). `InputError`/`AuthError` usages are verbose-tier. - Tighten baseline rules to one-liners; drop narrative phrasing. - Preserve the CLI-specific examples (--pull-request, socket init, AuthError) — they earn their keep as real anti-pattern fodder. Add `docs/references/error-messages.md` with fleet-wide worked examples so CLAUDE.md stays tight and the rich anti-patterns live once. * docs(claude): reference joinAnd / joinOr helpers in Error Messages Point readers at @socketsecurity/lib/arrays' list-formatting helpers from CLAUDE.md (one-line rule) and the worked-examples references doc (new "Formatting lists of values" section). * chore: bump @socketsecurity/lib to 5.24.0 and adopt error helpers Fleet-wide migration to the caught-value helpers in @socketsecurity/lib/errors. - pnpm-workspace.yaml: catalog bump 5.21.0 → 5.24.0. - 18 src files under packages/cli/src: replace every `e instanceof Error ? e.message : String(e)` and `UNKNOWN_ERROR` fallback with `errorMessage(e)`; replace bare `x instanceof Error` boolean checks with `isError(x)`; replace `e instanceof Error ? e.stack : undefined` with `errorStack(e)`. - packages/cli/src/utils/error/errors.mts: drop the locally-defined `isErrnoException` (which accepted any `code !== undefined`) and re-export the library's stricter string-code variant. - packages/cli/src/commands/manifest/convert-{gradle,sbt}-to-maven.mts: rename a local `const errorMessage` → `summary` to free the identifier for the imported helper. - packages/cli/src/utils/telemetry/service.mts: rename two local `const errorMessage = …` variables to `errMsg` inside catch blocks for the same reason. - docs/references/error-messages.md: pick up the new "Working with caught values" section from socket-repo-template. Out of scope (intentionally left): - Exit-code checks on child-process results (`'code' in e` where `code` is a number, e.g. display.mts:257). `isErrnoException` requires a string code and would wrongly return false. - The local `getErrorMessage` / `getErrorMessageOr` helpers in errors.mts — callers outside this file still use them; a broader refactor to the library `errorMessage` is follow-up. Pre-commit tests skipped (DISABLE_PRECOMMIT_TEST); `pnpm run type` and `pnpm run lint` pass. * fix(cli): stop duplicating cause messages in display.mts loop Cursor bugbot flagged the while(currentCause) loop in displayError: it walks the .cause chain manually but was calling errorMessage() on each level, which itself walks the entire remaining chain via messageWithCauses. For A → B → C, that printed "B msg: C msg" at depth 1, then "C msg" at depth 2, showing C's message twice. Switch to reading `.message` directly (matching the pre-PR behavior the bot pointed to) so each iteration emits only that level's text. Fall back to `String(currentCause)` for non-Error nodes in the chain. Drop the now-unused `errorMessage` import. Reported on PR #1261. * test(cli): update debug.test for @socketsecurity/lib/errors 5.24 semantics The debugApiResponse test expected errorMessage('String error') to return the 'Unknown error' sentinel, matching the old local shim's behavior that treated any non-Error caught value as unusable. The catalog bump to @socketsecurity/lib 5.24 switched debug.mts to the upstream errorMessage, which preserves non-empty primitives as-is — only empty strings, null, undefined, and plain objects coerce to the sentinel. Assert on 'String error' to pin the current contract.
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 24, 2026
…, terminal) (#1260) * fix(cli): align utils/ miscellaneous error messages with 4-ingredient strategy Final PR in the error-message series. Covers everything not already touched by #1255-#1259: utils/basics, utils/config, utils/fs, utils/git, utils/npm, utils/promise, utils/terminal, and the flags module at the root of the CLI tree. Sources: - flags.mts: 2 throws (--max-old-space-size, --max-semi-space-size) — name the flag, show the offending value, suggest a concrete megabyte value. - utils/config.mts: 1 throw (SOCKET_CLI_CONFIG base64 decode) — explains the replacement-character symptom and how to re-encode. - utils/basics/vfs-extract.mts: 4 throws (SEA VFS extraction for Python + security tools) — name the missing paths, the exit codes, and point at the "rebuild the SEA binary" fix. - utils/promise/queue.mts: 1 throw (PromiseQueue concurrency guard) — show the offending value and suggest 4/8. - utils/npm/spec.mts: 1 throw (PURL conversion) — show the input, state what a valid npm spec looks like. - utils/git/operations.mts: 1 throw (git-not-on-PATH) — point at install and the local-path env-var override. - utils/git/gitlab-provider.mts: 2 throws (no token, PR creation after retries) — name the token scope, the retry count, the repo/head refs. - utils/fs/path-resolve.mts: 1 throw (npm path-walk iteration cap) — name the start path, current directory, and what usually causes the cycle (symlinks). - utils/terminal/iocraft.mts: 1 throw (native-module load failure) — show the underlying error and the offending platform/arch triple. Skipped (already informative): - github-provider.mts pass-through errors (forward inner CResult cause/message) - gitlab-provider.mts try/catch wrappers that call formatErrorWithDetail (inner error has context) - 'process.exit called' sentinel throws in npm/pnpm/yarn/with- subcommands paths (test harness re-raise markers, not user-facing) Tests updated: - test/unit/utils/promise/queue.test.mts (2 assertions) - test/unit/utils/npm/spec.test.mts (2 assertions) - test/unit/utils/git/gitlab-provider.test.mts (3 assertions) Full suite (343 files / 5225 tests) passes. Completes the series: #1255 (commands/) → #1256 (utils/dlx/) → #1257 (utils/update + utils/command/) → #1258 (env/ + constants/) → #1259 (test/) → this. * fix(cli): address Cursor bugbot findings on error messages Four issues flagged by Cursor bugbot on #1260: 1. (Medium) gitlab-provider.mts: error said 'check GL_TOKEN permissions' but the actual env var is GITLAB_TOKEN (as the same file's getGitLabToken confirms). Fixed to GITLAB_TOKEN. 2. (Medium) git/operations.mts: error suggested 'set SOCKET_CLI_GIT_PATH to point at a specific binary' — that env var is not read anywhere. Removed the false suggestion; kept the real fix (install git and put it on PATH) with package-manager examples. 3. (Low) terminal/iocraft.mts: '(e as Error).message' evaluates to undefined when a non-Error is thrown. Switched to 'e instanceof Error ? e.message : String(e)' for safe stringification. 4. (Low) gitlab-provider.mts: error said 'after ${retries} retries' but the loop runs attempt 1..retries inclusive — retries is the total attempt count, not retries beyond the first. Reworded to 'attempts'. Matching test assertions updated. Caught by #1260 bugbot review. * chore(cli): use joinAnd + getErrorCause helpers in utils/ misc - basics/vfs-extract.mts: missingTools list now renders as prose via joinAnd('a, b, and c'). - terminal/iocraft.mts: inline `e instanceof Error ? e.message : String(e)` swapped for getErrorCause(e). require() of a native binding can throw non-Error values, so the safe-stringify with UNKNOWN_ERROR fallback is correct here. No behavior change for Error throws.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Align error messages in
packages/cli/src/utils/dlx/with the 4-ingredient strategy (what / where / saw / fix) from CLAUDE.md (#1254).Scope
packages/cli/src/utils/dlx/resolve-binary.mtspackages/cli/src/utils/dlx/spawn.mtspackages/cli/src/utils/dlx/vfs-extract.mtspackages/cli/test/unit/utils/dlx/resolve-binary.test.mts(matching regex updates)Bugbot fix included: TDZ crash in
ensurePythonDlx— the retry-limit guard interpolatedlockFile/pythonDirbefore their `const` declarations. Fixed by hoisting the declarations above the guard (commit a766d3d).Related PRs (sibling error-message batches)
commands/*(14 files)utils/update/+utils/command/+ error library migration + CLAUDE.md doctrine (supersedes docs(claude): align Error Messages with fleet doctrine #1261)env/+constants/+ sea-build scriptsutils/miscTest plan